Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for type checked term_at<T> and term_at<T>(i) #1397

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

deathbeam
Copy link
Contributor

@deathbeam deathbeam commented Oct 13, 2024

First method is simple loop over terms to find term for T. Its slower than using index directly, but sometimes type safety is more preferred and when the cost is only on initialization.

Second method is same as term_at(i) but with type checked assert. For middle ground to still keep the safety but also performance.

This is mostly to mirror the C# binding PR where this is slightly more useful but maybe cpp would benefit too:

BeanCheeseBurrito/Flecs.NET#52

Also disclaimer, I dont rly work with cpp often so I might've easily missed something, but I made best effort to write some tests and at least run them and see if they pass :d

Example use:

Current method

world.System<Position, Velocity>()
  .term_at(1)
  .singleton();

Type checked (throws assert if term_at(1) isnt Velocity type)

world.System<Position, Velocity>()
  .term_at<Velocity>(1)
  .singleton();

Type checked + resolved (throws assert if there is no Velocity component):

world.System<Position, Velocity>()
  .term_at<Velocity>()
  .singleton();

@deathbeam deathbeam force-pushed the type-checked-term-at branch 2 times, most recently from 5a80df7 to ac73c71 Compare October 13, 2024 05:44
@copygirl
Copy link
Contributor

Would it make sense to have the parameter-less term_at named differently?

@deathbeam
Copy link
Contributor Author

Maybe just .term<>() and then also add .term(flecs::entity_t component) to match other apis?

@deathbeam deathbeam force-pushed the type-checked-term-at branch from ac73c71 to 0a9e0af Compare October 14, 2024 10:49
@deathbeam deathbeam changed the title Add support for type checked term_at<T> and term_at<T>(i) Add support for type checked term<T> and term_at<T>(i) Oct 14, 2024
@deathbeam deathbeam force-pushed the type-checked-term-at branch from 0a9e0af to c1b556a Compare October 14, 2024 10:52
@deathbeam
Copy link
Contributor Author

Renamed term_at to term. Also updated the logic to account for pairs properly

@deathbeam deathbeam force-pushed the type-checked-term-at branch 3 times, most recently from 72e7ea1 to fe2c902 Compare October 14, 2024 11:58
@deathbeam
Copy link
Contributor Author

Looks like I cant use term as it fails on build here so changing back to term_at I guess:

https://github.com/SanderMertens/flecs/actions/runs/11327040474/job/31497292704?pr=1397

@deathbeam deathbeam force-pushed the type-checked-term-at branch from fe2c902 to d9a0a6d Compare October 14, 2024 12:22
@deathbeam deathbeam changed the title Add support for type checked term<T> and term_at<T>(i) Add support for type checked term_at<T> and term_at<T>(i) Oct 14, 2024
@deathbeam deathbeam force-pushed the type-checked-term-at branch from d9a0a6d to 7adca4c Compare October 15, 2024 19:20
First method is simple loop over terms to find term for T. Its slower
than using index directly, but sometimes type safety is more preferred
and when the cost is only on initialization.

Second method is same as term_at(i) but with type checked assert. For
middle ground to still keep the safety but also performance.

This is mostly to mirror the C# binding PR where this is slightly more
useful but maybe cpp would benefit too:

BeanCheeseBurrito/Flecs.NET#52

Signed-off-by: Tomas Slusny <[email protected]>
@deathbeam deathbeam force-pushed the type-checked-term-at branch from 7adca4c to ad60d47 Compare October 15, 2024 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants